-
Notifications
You must be signed in to change notification settings - Fork 89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rescue extract-xiso from oblivion #80
base: master
Are you sure you want to change the base?
Conversation
c572379
to
86bda64
Compare
They are processed differently though. My understanding (but it has been a while since I've looked at the XISO filesystem driver):
|
I converted the PR to draft because I want to make some more checks just to make sure, and eventually solve some other problems. |
3f09655
to
841e4ed
Compare
bd4d9ce
to
64a781b
Compare
I think I'm done for now |
9f85125
to
9378b67
Compare
I implemented what I called the "discover strategy", basically the linear listing of files in a directory. It is used when listing or extracting by default. The rewrite mode, instead uses both strategies: first processes the tree, then lists the directories sequentially, hoping to find "hidden" files. I wanted a single strategy for all cases, but unfortunately it's not possible unless we generate the tree also for listing and extracting. The rewriting is done in two separate steps: first tree discovery, then linear search. Doing both at the same time would fail spectacularly on malformed ISOs, and introduce a lot of overhead even in normal circumstances. This means that:
I don't know if you like it, or if changing the default strategy for file listing and extraction is the right thing to do, or even if it is useful at all. Let me know your feedback. |
1da0279
to
afeff82
Compare
Fix file size test in verify_xiso() broken since 715d798
Move to 'softprops/action-gh-release@v1' action Update 'actions/checkout' to v4
Replace invalid CP1252 characters with question mark
Symlinks are not followed below the root, to avoid potential recursions Windows is an exception, since we don't have an 'lstat' equivalent
The new code is licensed under BSD
Now symlinks are skipped on Windows too
d0991a1
to
ffaf8a7
Compare
Remove unused defines
Disable -s option in listing mode
Replace fors with whiles when appropriate Small cleanups
I think that this tool still has a lot of potential, and doesn't deserve to be abandoned, so I decided to rewrite
traverse_xiso
, the most difficult function to read and understand in the code, and split it into two pieces to make it easier to understand.Judging from the changelog on top of the file, it appears that it once used recursion, but it has been rewritten as the spaghetti code is it now to avoid sporadic stack overflows with highly unoptimized ISOs written by some tools.
I honestly don't think the cause of the overflows was the recursion: each iteration uses very little memory and the same ISOs are processed on the OG Xbox anyways. Probably it was some other bug that manifested itself only in some cases.
I tried to keep the memory usage as low as possible, by freeing up unused memory early, but it will probably use more memory than before. I guess that's the tradeoff for code legibility.
Anyways, the code has been tested to work and produce functionally identical ISOs. They are not hash-identical to the previous ones because of the order the files are processed. As as side note, it looks like that now it generates node trees identical to retail, so I guess it is more accurate? Anyways, if someone knows what tools produce such linked list ISOs, it would be awesome to test them and see if the stack overflow problem is fixed.
Of course it needs extensive testing, but I think that with
traverse_xiso
rewritten, it would be much easier to update the tool and/or solve bugs.The next step would be to review all the open issues, and see if they still apply. In particular, I'm using 64-bit builds and they produce files identical to 32-bit ones, so maybe this particular issue is solved.